Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Dec 10, 2025

Summary

  • filter out empty params to ensure LLM can set tool params at runtime
    • previously, if a user set a field and then removed it, then the agent wouldn't be able to set that param at runtime since it'd assume its already set (because in the state typing and removing makes it '' instead of undefined)

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 10, 2025 7:19pm

@waleedlatif1 waleedlatif1 marked this pull request as ready for review December 10, 2025 19:31
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 10, 2025

Greptile Overview

Greptile Summary

Fixes a bug where user-cleared form fields (empty strings '') would override LLM-generated parameter values. When users set a tool parameter and then clear it, the UI state stores '' instead of undefined. Previously, these empty strings would take precedence over LLM values during parameter merging.

  • apps/sim/providers/utils.ts: prepareToolExecution now filters out empty, null, and undefined values from tool.params before merging with LLM args
  • apps/sim/tools/params.ts: mergeToolParameters now applies the same filtering, making it consistent with the existing filterSchemaForLLM function
  • apps/sim/tools/params.test.ts: Added two test cases validating the new filtering behavior for empty strings, null, and undefined values

The fix is consistent with the existing filterSchemaForLLM function which already had this filtering logic. The approach ensures that when a user explicitly clears a field, the LLM can still provide a value at runtime.

Confidence Score: 5/5

  • This PR is safe to merge - it's a targeted bug fix with consistent implementation and good test coverage.
  • The changes are minimal, focused, and follow an existing pattern already used in filterSchemaForLLM. The fix addresses a clear bug without introducing side effects. New tests validate the behavior.
  • No files require special attention.

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/providers/utils.ts 5/5 Adds filtering logic in prepareToolExecution to skip empty/null/undefined user params, ensuring LLM-generated values aren't overridden by cleared fields.
apps/sim/tools/params.ts 5/5 Adds same filtering logic to mergeToolParameters function, making it consistent with filterSchemaForLLM which already had this behavior.
apps/sim/tools/params.test.ts 5/5 Adds two new test cases for mergeToolParameters: one for empty strings and one for null/undefined values. Good coverage for the new behavior.

Sequence Diagram

sequenceDiagram
    participant UI as User Interface
    participant State as UI State
    participant PrepTool as prepareToolExecution
    participant MergeTool as mergeToolParameters
    participant LLM as LLM Agent

    UI->>State: User sets field value
    UI->>State: User clears field (becomes '')
    State->>PrepTool: tool.params (includes '')
    LLM->>PrepTool: llmArgs (generated values)
    PrepTool->>PrepTool: Filter out empty/null/undefined
    PrepTool->>PrepTool: Merge filtered params with llmArgs
    Note over PrepTool: LLM values preserved for cleared fields
    
    alt Custom Tool Path
        State->>MergeTool: userProvidedParams (includes '')
        LLM->>MergeTool: callParams (generated values)
        MergeTool->>MergeTool: Filter out empty/null/undefined
        MergeTool->>MergeTool: Merge filtered params with callParams
        Note over MergeTool: LLM values preserved for cleared fields
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 163db5c into staging Dec 10, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/mcp-tool-inp branch December 10, 2025 19:38
waleedlatif1 added a commit that referenced this pull request Dec 10, 2025
* feat(folders): add the ability to create a folder within a folder in popover (#2287)

* fix(agent): filter out empty params to ensure LLM can set tool params at runtime (#2288)

* fix(mcp): added backfill effect to add missing descriptions for mcp tools (#2290)

* fix(redis): cleanup access pattern across callsites (#2289)

* fix(redis): cleanup access pattern across callsites

* swap redis command to be non blocking

* improvement(log-details): polling, trace spans (#2292)

---------

Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: Emir Karabeg <78010029+emir-karabeg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants